Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE #8486]Add more test coverage for BrokerMetricsManager #8487

Merged
merged 4 commits into from
Aug 7, 2024

Conversation

ziiyee
Copy link
Contributor

@ziiyee ziiyee commented Aug 3, 2024

Which Issue(s) This PR Fixes

Fixes #8486

Brief Description

Add more test coverage.

Before:
Pasted Graphic 2

After:
Pasted Graphic 1

How Did You Test This Change?

All tests passed.

image

PS: TONGYI Lingma provided code generation and programming support.
通义灵码使用-如何使用测试覆盖率插件

- check the topic is retry or dlq topic
- check the group is system group or not
- check the topic and the group belongs to system or not
@ziiyee
Copy link
Contributor Author

ziiyee commented Aug 3, 2024

for soc in #8262

Copy link
Member

@TheR1sing3un TheR1sing3un left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM~
Thanks for your contributions!

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.46%. Comparing base (2ed4ba2) to head (62913cc).
Report is 1 commits behind head on develop.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #8487      +/-   ##
=============================================
+ Coverage      46.01%   46.46%   +0.45%     
- Complexity     11144    11200      +56     
=============================================
  Files           1274     1274              
  Lines          89008    89022      +14     
  Branches       11444    11448       +4     
=============================================
+ Hits           40958    41368     +410     
+ Misses         42925    42526     -399     
- Partials        5125     5128       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ziiyee
Copy link
Contributor Author

ziiyee commented Aug 4, 2024

Thank you~
I'm a newcomer to open source development, should I fix the bazel-compile build errors to get this PR merged?
@TheR1sing3un

@ShannonDing ShannonDing added the soc Summer of Code, hosted by Google, Alibaba, Chinese Academy of Sciences and so on label Aug 5, 2024
@TheR1sing3un
Copy link
Member

Thank you~ I'm a newcomer to open source development, should I fix the bazel-compile build errors to get this PR merged? @TheR1sing3un

Bazel-complie check isn't required for pr merging, but u can try to fix it in a new pr.

@ShannonDing ShannonDing changed the title Add more test coverage for BrokerMetricsManager [ISSUE #8486]Add more test coverage for BrokerMetricsManager Aug 5, 2024
@ShannonDing
Copy link
Member

@ziiyee A reminder from the Tianchi platform: it‘s better to submit your results to the official website asap.

@ziiyee
Copy link
Contributor Author

ziiyee commented Aug 5, 2024

@ziiyee A reminder from the Tianchi platform: it‘s better to submit your results to the official website asap.

Thanks a lot, it's a good event.
Could you please merge this PR so I can submit my results?
@ShannonDing

Copy link
Member

@ShannonDing ShannonDing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ShannonDing ShannonDing merged commit cba8a64 into apache:develop Aug 7, 2024
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
soc Summer of Code, hosted by Google, Alibaba, Chinese Academy of Sciences and so on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Add more test coverage for BrokerMetricsManager
4 participants